Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix handling of the undefined host type for stream errors #4052

Merged
merged 1 commit into from
Jul 4, 2023
Merged

Fix handling of the undefined host type for stream errors #4052

merged 1 commit into from
Jul 4, 2023
+1 −1

Conversation

chrzaszcz
Copy link
Member

@chrzaszcz chrzaszcz commented Jul 3, 2023 •
edited
Loading

Stream errors are often sent before the host type is known. The host_type in the c2s state is then undefined, not <<>>.
Because of this bug, the following errors could be seen in the logs:

when=2023-07-03T12:30:17.494116+00:00 level=error what=undefined_host_type pid=<0.2124.0> at=mongoose_hooks:run_hook_for_host_type/4:1373 text="Running hook for an undefined host type" hook_name=xmpp_send_element hook_args_el="{xmlel,<<\"stream:error\">>,[],[{xmlel,<<\"xml-not-well-formed\">>,[{<<\"xmlns\">>,<<\"urn:ietf:params:xml:ns:xmpp-streams\">>}],[]}]}" hook_acc_{c2s,send_result}=ok hook_acc_timestamp=1688387417491894 hook_acc_statem_acc_state_mod=... hook_acc_statem_acc_socket_send= hook_acc_statem_acc_hard_stop=undefined hook_acc_statem_acc_c2s_state=undefined hook_acc_statem_acc_c2s_data=undefined hook_acc_statem_acc_actions= hook_acc_stanza_type=undefined hook_acc_stanza_to_jid=undefined hook_acc_stanza_ref=#Ref<0.1048366250.1175977986.191846> hook_acc_stanza_name=stream:error hook_acc_stanza_from_jid="{jid,<<>>,<<\"localhost\">>,<<>>}" hook_acc_stanza_element="{xmlel,<<\"stream:error\">>,[],[{xmlel,<<\"xml-not-well-formed\">>,[{<<\"xmlns\">>,<<\"urn:ietf:params:xml:ns:xmpp-streams\">>}],[]}]}" hook_acc_ref=#Ref<0.1048366250.1175977986.191847> hook_acc_origin_pid=<0.2124.0> hook_acc_origin_location_mfa={mongoose_c2s,send_element_from_server_jid,2} hook_acc_origin_location_line=871 hook_acc_origin_location_file=/Users/pawelchrzaszcz/dev/mongoose/src/c2s/mongoose_c2s.erl hook_acc_non_strippable= hook_acc_mongoose_acc=true hook_acc_lserver=localhost hook_acc_host_type=undefined

By fixing this line:

@chrzaszcz
Fix handling of the undefined host type for stream errors
3f3dae4 
Stream errors are often sent before the host type is known.
The 'host_type' in the c2s state is then undefined, not '<<>>'.

By fixing this line:
- There are no more errors in the logs about running hooks for
  undefined host type.
- The line should be covered by the tests now.
@mongoose-im
Copy link
Collaborator

mongoose-im commented Jul 3, 2023 •
edited
Loading

small_tests_25_arm64 / small_tests / 3f3dae4
Reports root / small

small_tests_24 / small_tests / 3f3dae4
Reports root / small

small_tests_25 / small_tests / 3f3dae4
Reports root / small

ldap_mnesia_24 / ldap_mnesia / 3f3dae4
Reports root/ big
OK: 2249 / Failed: 0 / User-skipped: 826 / Auto-skipped: 0

dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 3f3dae4
Reports root/ big
OK: 4211 / Failed: 0 / User-skipped: 82 / Auto-skipped: 0

dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / 3f3dae4
Reports root/ big
OK: 4211 / Failed: 0 / User-skipped: 82 / Auto-skipped: 0

ldap_mnesia_25 / ldap_mnesia / 3f3dae4
Reports root/ big
OK: 2249 / Failed: 0 / User-skipped: 826 / Auto-skipped: 0

dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / 3f3dae4
Reports root/ big
OK: 4208 / Failed: 0 / User-skipped: 85 / Auto-skipped: 0

dynamic_domains_mysql_redis_25 / mysql_redis / 3f3dae4
Reports root/ big
OK: 4185 / Failed: 0 / User-skipped: 108 / Auto-skipped: 0

pgsql_mnesia_24 / pgsql_mnesia / 3f3dae4
Reports root/ big
OK: 4594 / Failed: 0 / User-skipped: 89 / Auto-skipped: 0

elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / 3f3dae4
Reports root/ big
OK: 2755 / Failed: 0 / User-skipped: 659 / Auto-skipped: 0

internal_mnesia_25 / internal_mnesia / 3f3dae4
Reports root/ big
OK: 2394 / Failed: 1 / User-skipped: 680 / Auto-skipped: 0

pubsub_SUITE:tree+node_config:notify_only_available_users_test 
{error,{{badmatch,false},
    [{pubsub_tools,check_response,2,
             [{file,"/home/circleci/project/big_tests/tests/pubsub_tools.erl"},
            {line,444}]},
     {pubsub_tools,receive_response,3,
             [{file,"/home/circleci/project/big_tests/tests/pubsub_tools.erl"},
            {line,434}]},
     {pubsub_tools,receive_and_check_response,4,
             [{file,"/home/circleci/project/big_tests/tests/pubsub_tools.erl"},
            {line,424}]},
     {pubsub_SUITE,'-notify_only_available_users_test/1-fun-0-',2,
             [{file,"/home/circleci/project/big_tests/tests/pubsub_SUITE.erl"},
            {line,847}]},
     {escalus_story,story,4,
            [{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
             {line,72}]},
     {test_server,ts_tc,3,[{file,"test_server.erl"},{line,1782}]},
     {test_server,run_test_case_eval1,6,
            [{file,"test_server.erl"},{line,1291}]},
     {test_server,run_test_case_eval,9,
            [{file,"test_server.erl"},{line,1223}]}]}}

Report log

mysql_redis_25 / mysql_redis / 3f3dae4
Reports root/ big
OK: 4580 / Failed: 0 / User-skipped: 103 / Auto-skipped: 0

pgsql_mnesia_25 / pgsql_mnesia / 3f3dae4
Reports root/ big
OK: 4594 / Failed: 0 / User-skipped: 89 / Auto-skipped: 0

mssql_mnesia_25 / odbc_mssql_mnesia / 3f3dae4
Reports root/ big
OK: 4591 / Failed: 0 / User-skipped: 92 / Auto-skipped: 0

internal_mnesia_25 / internal_mnesia / 3f3dae4
Reports root/ big
OK: 2395 / Failed: 0 / User-skipped: 680 / Auto-skipped: 0

@codecov
Copy link

codecov bot commented Jul 3, 2023 •
edited
Loading

Codecov Report

Patch and project coverage have no change.

Comparison is base (6021247) 83.80% compared to head (3f3dae4) 83.80%.

Additional details and impacted files 
@@           Coverage Diff           @@
##           master    #4052   +/-   ##
=======================================
  Coverage   83.80%   83.80%           
=======================================
  Files         526      526           
  Lines       33129    33129           
=======================================
+ Hits        27764    27765    +1     
+ Misses       5365     5364    -1
Impacted Files Coverage Δ
src/c2s/mongoose_c2s.erl 87.71% <ø> (+0.43%) ⬆️

... and 10 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@chrzaszcz chrzaszcz marked this pull request as ready for review July 3, 2023 14:57
DenysGonchar
DenysGonchar approved these changes Jul 4, 2023
View reviewed changes
Copy link
Collaborator

@DenysGonchar DenysGonchar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@DenysGonchar DenysGonchar merged commit 8b12212 into master Jul 4, 2023
@DenysGonchar DenysGonchar deleted the fix-undefined-host-type branch July 4, 2023 08:45
@chrzaszcz chrzaszcz added this to the 6.2.0 milestone Dec 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DenysGonchar DenysGonchar DenysGonchar approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
6.2.0
Development

Successfully merging this pull request may close these issues.
3 participants
@chrzaszcz @mongoose-im @DenysGonchar